Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Error handling after increasing the array index #280

Merged
merged 2 commits into from
Oct 25, 2022
Merged

Error handling after increasing the array index #280

merged 2 commits into from
Oct 25, 2022

Conversation

vargajb
Copy link
Contributor

@vargajb vargajb commented Oct 19, 2022

Signed-off-by: Janos Varga 113785741+vargajb@users.noreply.github.com

Proposed changes

  • After increasing the array index, an error handling must be included, because if the input is extended later again, we can look for the error again. A handled error is easier to find and fix than a buffer overwrite error.
  • Table index should start from 0, because it indicates better that the table is empty.

Fixes # (issue)

Type of change

What type of changes does your PR introduce to the COBOL Programming Course? Put an x in the boxes that apply.

  • Bug fix (change which fixes one or more issues)
  • New feature (change which adds functionality or features to the course)
  • Translations (change which adds or modifies translations of the course)
  • Documentation (change which modifies documentation related to the course)
  • This change requires an update to the course's z/OS environment

Checklist:

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This checklist will be used as reference for both the contributor and the reviewer

  • I have read the Contributing Guideline
  • I have included a title and description for this PR
  • I have DCO-signed all of my commits that are included in this PR
  • I have tested it manually and there are no regressions found
  • I have commented my code, particularly in hard-to-understand areas (if appropriate)
  • I have made corresponding changes to the documentation (if appropriate)

@vargajb
Error handling after increasing the array index
697cde4 
After increasing the array index, an error handling must be included, because if the input is extended later again, we can look for the error again. A handled error is easier to find and fix than a buffer overwrite error.
Table index should start from 0, because it indicates better that the table is empty.

Signed-off-by: Janos Varga <113785741+vargajb@users.noreply.github.com>
@tanto259
Copy link
Member

I'm technically okay with merging this. However, this code is part of the COBOL Debugging Challenge on the Advanced Topic chapter, and I'm not the one who made the challenge. So, tagging @jellypuno here to see if she has any feedbacks about it.

@tanto259 tanto259 added the enhancement New feature or request label Oct 19, 2022
@vargajb
Copy link
Contributor Author

vargajb commented Oct 20, 2022

Hi @jellypuno,

you made the COBOL Debugging Challenge on the Advanced Topic of Cobol Programming Course in Openmainframeproject.
I would make some improvements in this challenge.

  • Before increasing the table "OVERLIMIT", I would make an error handling to check if there is enough space available in the table. This way it would be safer to fill the array. If the input is extended later again, we can look for the error again. A handled error is easier to find and fix than a buffer overwrite error.
    My suggestion would be the following:
       IS-OVERLIMIT.
           IF ACCT-LIMIT < ACCT-BALANCE THEN
               ADD 1 TO SUB1
      * Check if there is enough space in the array, in case the input 
      * file changes again. A handled error is easier to find and fix 
      * than a buffer overwrite error.
               IF SUB1 > OVERLIMIT-MAX THEN
                   DISPLAY 'OVERFOLW TABLE OVERLIMIT'
                   MOVE 1000 TO RETURN-CODE
                   STOP RUN
               END-IF
               MOVE ACCT-LIMIT TO OL-ACCT-LIMIT(SUB1)
               MOVE ACCT-BALANCE TO OL-ACCT-BALANCE(SUB1)
               MOVE LAST-NAME TO OL-LASTNAME(SUB1)
               MOVE FIRST-NAME TO OL-FIRSTNAME(SUB1)
            END-IF.
  • I would start the table index from 0, because it indicates better that the table is empty.

I tested the changes.
We would need your opinion about this before merging this pull request.

Thanks!

BR:
János
cc: @tanto259

@vargajb
Copy link
Contributor Author

vargajb commented Oct 25, 2022

Hi @tanto259,

we didn’t get any feedback from @jellypuno. She seems to be very busy.

What is my correction about?
Why should a storage error be detected as quickly as possible?

  • Because unhandled storage errors are much more difficult to find than handled errors.
  • In a normal production environment, programs with more than 100,000 lines are completely normal, and thus debugging is much more difficult.
  • It is not always easy to reproduce the storage space problem in a test environment.
  • It is not without precedent that a storage space problem has been fixed by itself after inserting the DISPLAY statement for debugging. In such a case, you can write a comment in the program saying: "Do not remove this DISPLAY, otherwise the program will crash".

Should I explain my correction in more detail?
Would you suggests a better correction for this issue?

Thanks!

BR:
János

tanto259
tanto259 requested changes Oct 25, 2022
View reviewed changes
Copy link
Member

@tanto259 tanto259 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'll just merge this first after the typo is fixed. I personally have no objections on the edit. If she have any feedbacks, we can always make changes later on.

COBOL Programming Course #3 - Advanced Topics/Challenges/Debugging/cbl/CBL0106C.cbl Outdated Show resolved Hide resolved
@vargajb
Typo correction
0bf1f35 
Signed-off-by: Janos Varga <113785741+vargajb@users.noreply.github.com>
@tanto259 tanto259 merged commit 8cba38b into openmainframeproject:master Oct 25, 2022
@tanto259
Copy link
Member

Thanks, @vargajb!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tanto259 tanto259 tanto259 approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vargajb @tanto259